-
-
Notifications
You must be signed in to change notification settings - Fork 460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(lint/noStaticElementInteractions): add rule #2981
feat(lint/noStaticElementInteractions): add rule #2981
Conversation
CodSpeed Performance ReportMerging #2981 will not alter performanceComparing Summary
|
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
As this comment states, the following two methods were created to bridge the gap between eslint-plugin-jsx-a11y and Biome
However, I thought it was necessary to optimize the Biome definition based on the official documentation, rather than discussing the two options of which way to go. Is this an issue that should be addressed in this PR? |
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
Let's address that in another PR. |
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
@unvalley |
crates/biome_js_analyze/src/lint/nursery/no_static_element_interactions.rs
Outdated
Show resolved
Hide resolved
Of course, helps are always welcome 😄 |
@ryo-ebata could you apply codegen to pass the CI? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some suggestions but they are nits or for refactor. Thank you!
@unvalley
|
@ryo-ebata There is a regression in useValidAutocomplete/valid.jsx.snap, could you take a look |
Okay! I'll address in this PR! |
@unvalley But one concern has arisen. If an attribute value is dynamic, the caller of extract_attributes() must now handle the processing differently. Therefore, we want to return a value from extract_attributes() that indicates whether it's a dynamic value, but since the return type is String, we're currently returning "[DYNAMIC]". However, if a user actually sets "[DYNAMIC]" as an attribute value, the system will incorrectly interpret it as a dynamic value. I'd like to consult with you on whether there might be a better approach to this issue.... |
@ryo-ebata This PR is getting longer, so if this takes too long, update the test case and that change is fine in another PR. |
08adc60
to
201af4f
Compare
…thub.com/ryo-ebata/biome into ryo-ebata/no-static-element-interactions
…lement-interactions
Summary
Enforce that non-interactive, visible elements (such as
<div>
) that have click handlers use the role attribute.Implements #527
Test Plan
Added snaps for valid/invalid cases.
Basically, it conforms to the test cases of eslint-plugin-jsx-a11y/no-static-element-interactions, and test cases that cannot be handled by the resources within Biome are handled individually.
ref:
https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/__tests__/src/rules/no-static-element-interactions-test.js